Skip to content

doc: fix CCM cipher example in MJS#39949

Closed
tniessen wants to merge 3 commits into
nodejs:masterfrom
tniessen:doc-fix-crypto-ccm-mjs
Closed

doc: fix CCM cipher example in MJS#39949
tniessen wants to merge 3 commits into
nodejs:masterfrom
tniessen:doc-fix-crypto-ccm-mjs

Conversation

@tniessen

Copy link
Copy Markdown
Member

The original example used 'return' to terminate the current control
flow, which is valid in CommonJS. When the example was copied and
modified to use MJS syntax, the 'return' statement was left in but is
not allowed.

Refs: #37594

The original example used 'return' to terminate the current control
flow, which is valid in CommonJS. When the example was copied and
modified to use MJS syntax, the 'return' statement was left in but is
not allowed.

Refs: nodejs#37594
@nodejs-github-bot nodejs-github-bot added crypto Issues and PRs related to the crypto subsystem. doc Issues and PRs related to the documentations. labels Aug 30, 2021

@aduh95 aduh95 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might as well use the Error Cause proposal which is now implemented by V8.

Comment thread doc/api/crypto.md Outdated
Comment thread doc/api/crypto.md Outdated
tniessen and others added 2 commits September 2, 2021 15:19
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
@tniessen

tniessen commented Sep 2, 2021

Copy link
Copy Markdown
Member Author

@aduh95 No strong opinion, done. No current Node.js release supports error causes as far as I can tell.

@aduh95

aduh95 commented Sep 2, 2021

Copy link
Copy Markdown
Contributor

No current Node.js release supports error causes as far as I can tell.

Next Node.js v16.x will, as V8 9.3 has been backported (https://v8.dev/blog/v8-release-93#error-cause).

@tniessen tniessen added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 10, 2021
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 10, 2021
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

aduh95 pushed a commit that referenced this pull request Sep 10, 2021
The original example used 'return' to terminate the current control
flow, which is valid in CommonJS. When the example was copied and
modified to use MJS syntax, the 'return' statement was left in but is
not allowed.

Refs: #37594

PR-URL: #39949
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
@aduh95

aduh95 commented Sep 10, 2021

Copy link
Copy Markdown
Contributor

Landed in 00ca848

@aduh95 aduh95 closed this Sep 10, 2021
BethGriggs pushed a commit that referenced this pull request Sep 21, 2021
The original example used 'return' to terminate the current control
flow, which is valid in CommonJS. When the example was copied and
modified to use MJS syntax, the 'return' statement was left in but is
not allowed.

Refs: #37594

PR-URL: #39949
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
BethGriggs pushed a commit that referenced this pull request Sep 21, 2021
The original example used 'return' to terminate the current control
flow, which is valid in CommonJS. When the example was copied and
modified to use MJS syntax, the 'return' statement was left in but is
not allowed.

Refs: #37594

PR-URL: #39949
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
@BethGriggs BethGriggs mentioned this pull request Sep 21, 2021
1 task
@tniessen tniessen deleted the doc-fix-crypto-ccm-mjs branch October 7, 2021 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

crypto Issues and PRs related to the crypto subsystem. doc Issues and PRs related to the documentations.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants